fix(extract): receiver-qualify Go method QNs - #1913

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

Rebased onto current main (stacked on the rebased #1907). No semantic changes; full test leg green. Post-fix field numbers for this change are in the #1906 census comment (method-node recovery incl. generic receivers: #1906 (comment)).

Reviewed the last commit. The bug is real and badly worth fixing, the shape of the fix is right, and there is one measurement I want before it lands.

The problem is as bad as you say. store.c:257 declares UNIQUE(project, qualified_name) and the node upsert is ON CONFLICT(project, qualified_name) DO UPDATE SET, so a flat proj.pkg.method genuinely makes every same-name method in a package one row. Nine Task() methods fusing into a single node carrying all nine bodies' call edges is not a cosmetic count problem — that node is a chimera whose outgoing edges are attributed to code that never made them. And a _test.go mock outranking the production method in the dedupe tie-break is the kind of thing that quietly poisons every answer downstream.

Receiver-qualifying is the right cure, matching the Go interface-member and C++ out-of-line paths already in extract_func_def, and lifting go_receiver_type_name to a shared cbm_go_receiver_type_name so both sides of the contract compute one formula is exactly right — two copies of that formula would drift.

The measurement I want

You describe this as a side effect, in one line:

resolve_same_module's exact module.name hash no longer matches concrete methods, which removes the confidence-0.9 false edges where an interface-typed call bound to an unrelated same-package method.

I think that line is doing more work than it looks. resolve_same_module (registry.c:790) builds its candidate as module_qn + "." + callee_name. Before this change a call to Close inside proj.pkg hit proj.pkg.Close at confidence 0.90. After it, the method is proj.pkg.Storage.Close and that candidate misses — and the suffix retry at :795 builds the same shape, so it misses too.

So same_module stops resolving every Go concrete-method call, not only the wrong ones. The true ones do not disappear; they fall through to resolve_name_lookup, which picks a winner by import distance project-wide at a much lower confidence. That is the failure mode this repo has been bitten by before: a blocked high-confidence match can lose a correct edge and mint a wrong one in the same step, and the net can be negative even when every individual removal was justified.

You measured the false edges removed. I would like the other half measured too, and there is already an instrument for it — the per-language CALLS census with its dumped per-edge sets. The Go leg of that is the right baseline. Concretely, what I am asking for is:

• Go CALLS edge count before and after, and
• the strategy histogram — specifically how many edges moved out of same_module and where they landed.

If the answer is "the LSP type-dispatch path already covers them, so the demotions are few and the removals are many", that settles it and the PR merges on the strength of the numbers rather than on the argument. If a meaningful number of true edges demote to import-distance guessing, that is worth knowing before this is in main rather than after.

The lrp_go_s* probes staying green is real evidence and I am not dismissing it — but they are a fixed scenario set, and the population this affects is every Go method call in a repo without LSP coverage on the receiver.

Two smaller things

This shifts a baseline other PRs are being judged against. There is an open cluster of call-resolution PRs that we have decided to judge against one shared census precisely because whichever lands first makes the rest unreadable on their own numbers. A change to Go method QNs moves that baseline for the Go leg. Not an objection — just something I would rather sequence deliberately than discover afterwards.

Your reindex note is right and needs to travel further than the PR. A QN-shape change means existing graphs pick up the new shape only on rebuild, and that belongs in the release notes, not only in #1909. I will make sure it gets there.

Status

ms is DIRTY — this and both its stack neighbours (#1907, #1915) have conflicts. main was broken for a few hours today and was repaired an hour ago by #1993, so now is a good moment to rebase the stack.

Thanks for the measurements you did bring — "20 methods → 2 nodes, 19 structs linked to one shared node" is what turns this from a plausible tidy-up into an obvious defect.

Rebased onto current main as part of the 1907→1913→1915→1936 stack; no source conflicts, test anchors re-anchored. Full scripts/test.sh green on the stack head.

Here is the measurement — and it caught something real, so thank you for insisting on it.

The census (orca-runtime-sensor, Go→Go CALLS, isolated one-shot indexes)

Per-edge migration of #1907's 929 same_module keys (caller + callee-name identity, so chimera retargeting doesn't count as loss): 108 kept same_module (receiver still resolvable flat), 444 re-landed (308 field_type_hint, 97 lsp_strategy_cross_file, 39 unique_name), 377 lost entirely. The lost population is exactly the calls whose receiver cannot be typed — the ones same_module was binding at 0.90 by package-coincidence, which is this PR's thesis. The LSP strategies keep ≥96% of their edges and end above the pre-#1913 baseline. Net caller→callee pairs across the leg: +41.

What the first run of this census caught

My first measurement showed lsp_type_dispatch losing 568 edges and lsp_direct 105 — every sample was a method-body caller ((r *Repo) Install calling r.helper()). Root cause: go_lsp.c's process_function still set the enclosing-function QN to the bare package.func form; its own comment declared the contract ("must be the QN the textual call events and the defs pass produce"), and this PR had moved that shared form to package.Type.method on the other two sides but not this one. Every LSP resolution sourced from inside a method lost its caller join in cbm_pipeline_find_lsp_resolution and fell back — or died.

Amended into this PR (it is the same one-formula claim): process_function now receiver-qualifies via the shared cbm_go_receiver_type_name, with a reproduce-first pipeline test (pipeline_go_method_caller_keeps_lsp_join: same-receiver method call with a colliding decoy method in another package, RED without the go_lsp hunk, GREEN with it). The table above is measured with the fix; without it the leg read 14156 total with lsp_direct at 8263.

One upstream test adopted the new shape: parallel_go_cross_package_field_chain_resolves matched edges by the flat handler.PlaceOrder fragment; the edge it guards is intact — OrderHandler.PlaceOrder → OrderService.PlaceOrder, still lsp_type_dispatch at 0.95, now receiver-qualified on both endpoints (which also confirms the cross-package chain path under the join fix) — so its fragments moved to the qualified form.

On the baseline-shift point: agreed — these numbers are from the stack in its merge order (#1907 → #1913 → #1915 → #1936), so whichever leg lands first, the others' columns above stay the readable reference.

Full scripts/test.sh green on the amended stack head; branch force-pushed with the rebase.